Skip to content

Conversation

@iTrooz
Copy link

@iTrooz iTrooz commented Nov 5, 2025

What type of PR is this?

/kind feature

What this PR does / why we need it:

Two things:

  • make build flag --output=tar,dest=- have correct behaviour (streaming to stdout)
  • print error on build flag --output=type=something instead of outputting image to folder type=something (this is the case whether "something" is a valid type or not)

How to verify it

  • Run podman build . --output=tar,dest=-
  • Run podman build --output=type=something

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

make build flag `--output=tar,dest=-` have correct behaviour (streaming to stdout)
print error on build flag `--output=type=INVALID` instead of outputting image to folder `type=INVALID`

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 8, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iTrooz
Once this PR has been reviewed and has the lgtm label, please assign mheon for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@iTrooz iTrooz changed the title feat(build): make --output=tar,dest=- work + print error on build flag --output=type=something feat(build): make --output=tar,dest=- work Nov 8, 2025
@iTrooz iTrooz changed the title feat(build): make --output=tar,dest=- work feat(build): print error on build flag --output=type=something Nov 8, 2025
@iTrooz
Copy link
Author

iTrooz commented Nov 8, 2025

I updated the title with the most important change. Would you like me to split the PR into 2 instead ?

@iTrooz iTrooz force-pushed the build_output_flag branch from 1e9e1eb to 9e02ac7 Compare November 8, 2025 20:42
@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

2 similar comments
@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@iTrooz
Copy link
Author

iTrooz commented Nov 8, 2025

CI failed because of an image pull timeout. TestConformance/header-builtin succeeds locally. I think it should be re-run

@iTrooz iTrooz force-pushed the build_output_flag branch 3 times, most recently from a31731b to 5afe04d Compare November 13, 2025 09:17
@iTrooz
Copy link
Author

iTrooz commented Nov 14, 2025

I'm having trouble figuring what's wrong in my PR. The errors seem unrelated to my PR, happening in files that I did not modify at all. Could I get some help please @nalind ? Thank you in advance

@nalind
Copy link
Member

nalind commented Nov 14, 2025

The errors in the runc configurations are a known flake (#6205) that should be fixed in runc 1.3.2 or later, but our VM images don't include that yet, as prior to the recent round of security updates they probably had 1.3.0. The error on the testing farm looks like a transient network problem.
I've restarted them and will keep an eye on them.

Copy link
Member

@nalind nalind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for taking this on.

The changes to the BuildOutputOption type could be an API break. I do prefer the use of an enumerated type there, though, and while I don't see the current parsing function or type being used in either podman or openshift/builder, I still worry about breaking other consumers that are using the current parsing function and interpreting its results.

The type isn't used as member of any other exported APIs or structures, so, while it would definitely be a bigger patch, would you consider marking both the current structure and the current pkg/parse.GetBuildOutput() implementation as deprecated (optionally keeping the added error checks), defining a new BuildOutputOption structure (that looks like the updated one here) and parsing function in internal/parse, and having imagebuildah.stageExecutor.execute() and pkg/cli.GenBuildOptions() call the new function instead? I think that would let us keep your improvements without having to worry about breaking compilation for anyone who's calling the current API.

@iTrooz
Copy link
Author

iTrooz commented Nov 18, 2025

All done !
I ran tests I modified/added locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Categorizes issue or PR as related to a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants